-
Notifications
You must be signed in to change notification settings - Fork 688
Snapshot LSP #1505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Snapshot LSP #1505
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there anything left? I've been playing with this and can't break it, even with the race detector enabled.
I realized while gone that I forgot the file watchers for ATA locations. Working on that now. |
} | ||
|
||
func NewConfiguredProject( | ||
configFileName string, | ||
configFilePath tspath.Path, | ||
host ProjectHost, | ||
builder *projectCollectionBuilder, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NewConfiguredProject
is exported, but it takes a *projectCollectionBuilder
, which isn’t exported. That means code outside the package can’t actually call this function. Was the intent for the constructor to be internal (unexported), or for projectCollectionBuilder to be exported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions aren't referenced outside the package, I think, so it's somewhat innocuous given the internalness of the whole API (we have worse examples elsewhere... but usually not in this part of the code). Are you expecting to need to call any of these in tsgolint and that's the interest in having these available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two reasons really:
- from tsgolint, we aren't expecting to be able to call these, but it breaks how we generate our shims (we can always work around this so feel free to tell me to do that 🙂 ), as the inconsistency means that we're trying to reference private types.
- from a code pov, it doesn't really make sense for one to be public, and not, as it means
NewConfiguredProject
can't be called outside the package, despite it being exported?
That said, feel free to ignore these cmts and i can workaround in the tsgolint side.
// NewSnapshot | ||
func NewSnapshot( | ||
id uint64, | ||
fs *snapshotFS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment here, NewSnapshot
is public, but snapshotFS
is not?
Same comment with extendedConfigCache
on L45
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏
@camc314 I'm merging now but happy to follow up with changes. To answer your questions broadly, the change most aligned with my intentions would be to make |
…d types (#128) microsoft/typescript-go#1505 makes some changes that mean that some functions reference types that aren’t exported, this means that we can’t compile tsgolint anymore 😥 this PR skips generating shims for functions with unexported types, logging a warning to stderr.
Note
Diagrams are generated by Copilot with Python; they’re pretty rough but get the idea across. I used them for an internal team presentation and figured they’re still better than nothing for this.
Summary
This PR replaces the project system backing the LSP server with an immutable snapshot-based architecture.
Problem Statement
The original LSP implementation here in typescript-go inherited microsoft/TypeScript's architecture, which relied on mutable data structures suitable for synchronous operation. However, as we started to parallelize request handling, this approach became unsustainable. We were starting to spend a lot of time chasing down data races, for which the solution was often tacking on yet another mutex for an individual component.
Design Goals
The new architecture addresses these issues through immutable snapshots that can be safely read by multiple goroutines and efficiently cloned to create new state. This enables:
Architecture
Core Concepts
The system centers around snapshots - immutable representations of LSP server state at a point in time. Snapshots can be read concurrently without coordination and cloned to produce new snapshots when state changes are needed.
While TypeScript programs were already immutable, additional components needed to be refactored for immutability:
System Structure
The architecture separates concerns between mutable session management and immutable snapshot state:
Session Layer (Mutable)
Snapshot Layer (Immutable)
State Transitions
State changes occur through snapshot cloning:
Request Processing
When an LSP request needs to invoke a language service operation for a document, we first check if there are any pending changes that need to be applied (e.g. file watchers have been triggered, open files have been changed, or an ATA request has completed). If so, the Session's current snapshot is cloned to incorporate the pending changes while simultaneously ensuring that the default project for the requested file is loaded and up-to-date. If there are no pending changes, we check if the Session's current snapshot is capable of serving that request (that is, if a default project can be found for that file and the project is up-to-date). If not, we clone the snapshot, in this case with no file changes, but a request to load or update the default project for the requested file. Finally, we return a language service that uses the project from the latest snapshot.
Notable behavior changes
references
array, and it terminates work as soon as higher priority matches have been ruled out. In order to support this early termination of work, we can no longer keep every project we loaded open, since that set is nondeterministic. Instead, we keep open only the projects that led us to the one that ultimately contained the target file. Example:/workspace/packages/foo/test.ts
/workspace/packages/foo/tsconfig.json
, doesn't contain the file, has two references:/workspace/packages/bar/tsconfig.json
/workspace/packages/baz/tsconfig.json
/workspace/tsconfig.json
, doesn't contain the file, has 1,000 referencestests.tsconfig.json
had a reference offoo.test.tsconfig.json
foo.test.tsconfig.json
contains the file at index 50foo.test.tsconfig.json
is the default project./workspace/foo.test.tsconfig.json
(the default project)/workspace/tests.tsconfig.json
/workspace/tsconfig.json
/workspace/packages/foo/tsconfig.json